Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix typing for graphql.resolver #4551

Draft
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ogenstad
Copy link
Contributor

@ogenstad ogenstad commented Oct 6, 2024

No description provided.

@ogenstad ogenstad added the ci/skip-changelog Don't include this PR in the changelog label Oct 6, 2024
@github-actions github-actions bot added the group/backend Issue related to the backend (API Server, Git Agent) label Oct 6, 2024
Copy link

codspeed-hq bot commented Oct 6, 2024

CodSpeed Performance Report

Merging #4551 will degrade performances by 19.2%

Comparing pog-graphql-resolver-typing (74ba089) with develop (cdb157a)

Summary

❌ 1 regressions
✅ 9 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark develop pog-graphql-resolver-typing Change
test_schemabranch_duplicate 302.6 µs 374.5 µs -19.2%

@ogenstad
Copy link
Contributor Author

ogenstad commented Oct 7, 2024

Looks like the errors here is that some of the resolvers state that the requested schema type is only a NodeSchema but this isn't correct as some of these instances also use a ProfileSchema, in one of these instances it requires a ProfileSchema or a NodeSchema however one of the code paths would break if anything other than a NodeSchema was used. I'm guessing that the performance degradation has to do with the call to isinstance(). Will set this aside for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/skip-changelog Don't include this PR in the changelog group/backend Issue related to the backend (API Server, Git Agent)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant